[BUG] [JAVA] [SPRING] Fix - Missing JSpecify @Nullable on fluent setter parameter for optional properties #23681#23688
Open
swapneswarsundarray wants to merge 5 commits intoOpenAPITools:masterfrom
Conversation
The openapi-yaml generator was using HashMap when building object-level examples from property examples, which caused unstable field ordering that didn't match the source spec declaration order. Switch to LinkedHashMap to preserve the order as defined in the OpenAPI spec. This improves readability in Swagger UI and other downstream tools that render the auto-generated examples. Add test to verify property order preservation.
…enAPITools#23664) The openapi-yaml generator was using HashMap when building object-level examples from property examples, which caused unstable field ordering that didn't match the source spec declaration order. Switch to LinkedHashMap to preserve the order as defined in the OpenAPI spec. This improves readability in Swagger UI and other downstream tools that render the auto-generated examples. Add test to verify property order preservation.
…ecify is enabled The Spring generator was missing @nullable annotations on fluent setter parameters when useJspecify is enabled, while regular setters and getters had them correctly applied. This created an inconsistent nullness contract that caused static analysis tools to flag passing null to fluent setters as errors, even though the underlying field is nullable. Changes: - Updated JavaSpring/pojo.mustache to use {{>nullableDataType}} instead of {{{datatypeWithEnum}}} for fluent setter parameters - Applied the same fix to inherited property fluent setters - Updated java-camel-server/pojo.mustache with the same fix (extends SpringCodegen) This ensures consistent nullness annotations across: - Fields: private @nullable String firstName; - Getters: public @nullable String getFirstName() - Regular setters: public void setFirstName(@nullable String firstName) - Fluent setters: public UserBase firstName(@nullable String firstName)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 645 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed. cubic prioritises the most important files to review.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/client/petstore/spring-http-interface-reactive-noResponseEntity/src/main/java/org/openapitools/model/AdditionalPropertiesClass.java">
<violation number="1" location="samples/client/petstore/spring-http-interface-reactive-noResponseEntity/src/main/java/org/openapitools/model/AdditionalPropertiesClass.java:304">
P1: Fluent setter now double-wraps JsonNullable input, so callers will store a JsonNullable value inside another JsonNullable instead of the intended underlying value.</violation>
</file>
<file name="samples/client/petstore/spring-http-interface/src/main/java/org/openapitools/model/AdditionalPropertiesClassDto.java">
<violation number="1" location="samples/client/petstore/spring-http-interface/src/main/java/org/openapitools/model/AdditionalPropertiesClassDto.java:306">
P1: Fluent setter re-wraps an already nullable value, breaking JsonNullable undefined/present semantics.</violation>
</file>
<file name="samples/client/petstore/spring-http-interface-useHttpServiceProxyFactoryInterfacesConfigurator/src/main/java/org/openapitools/model/AdditionalPropertiesClass.java">
<violation number="1" location="samples/client/petstore/spring-http-interface-useHttpServiceProxyFactoryInterfacesConfigurator/src/main/java/org/openapitools/model/AdditionalPropertiesClass.java:304">
P2: Fluent setter double-wraps JsonNullable input, losing undefined/present semantics for anytype2.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/java-camel-server/pojo.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/java-camel-server/pojo.mustache:158">
P1: Fluent setters now accept nullable input but still call `Optional.of(...)`, so passing null can throw `NullPointerException` for optional properties.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache:147">
P1: Fluent setter parameters now use the wrapped nullable/optional type, but the body wraps the argument again, causing invalid nested `JsonNullable`/`Optional` assignments.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| public AdditionalPropertiesClass anytype2(Object anytype2) { | ||
| public AdditionalPropertiesClass anytype2(JsonNullable<Object> anytype2) { |
Contributor
There was a problem hiding this comment.
P1: Fluent setter now double-wraps JsonNullable input, so callers will store a JsonNullable value inside another JsonNullable instead of the intended underlying value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/spring-http-interface-reactive-noResponseEntity/src/main/java/org/openapitools/model/AdditionalPropertiesClass.java, line 304:
<comment>Fluent setter now double-wraps JsonNullable input, so callers will store a JsonNullable value inside another JsonNullable instead of the intended underlying value.</comment>
<file context>
@@ -301,7 +301,7 @@ public void setAnytype1(@Nullable Object anytype1) {
}
- public AdditionalPropertiesClass anytype2(Object anytype2) {
+ public AdditionalPropertiesClass anytype2(JsonNullable<Object> anytype2) {
this.anytype2 = JsonNullable.of(anytype2);
return this;
</file context>
Comment on lines
+306
to
308
| public AdditionalPropertiesClassDto anytype2(JsonNullable<Object> anytype2) { | ||
| this.anytype2 = JsonNullable.of(anytype2); | ||
| return this; |
Contributor
There was a problem hiding this comment.
P1: Fluent setter re-wraps an already nullable value, breaking JsonNullable undefined/present semantics.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/spring-http-interface/src/main/java/org/openapitools/model/AdditionalPropertiesClassDto.java, line 306:
<comment>Fluent setter re-wraps an already nullable value, breaking JsonNullable undefined/present semantics.</comment>
<file context>
@@ -303,7 +303,7 @@ public void setAnytype1(@Nullable Object anytype1) {
}
- public AdditionalPropertiesClassDto anytype2(Object anytype2) {
+ public AdditionalPropertiesClassDto anytype2(JsonNullable<Object> anytype2) {
this.anytype2 = JsonNullable.of(anytype2);
return this;
</file context>
Suggested change
| public AdditionalPropertiesClassDto anytype2(JsonNullable<Object> anytype2) { | |
| this.anytype2 = JsonNullable.of(anytype2); | |
| return this; | |
| public AdditionalPropertiesClassDto anytype2(JsonNullable<Object> anytype2) { | |
| this.anytype2 = anytype2; | |
| return this; | |
| } |
Comment on lines
+158
to
160
| public {{classname}} {{name}}({{>nullableDataType}} {{name}}) { | ||
| {{#openApiNullable}} | ||
| this.{{name}} = {{#isNullable}}JsonNullable.of({{/isNullable}}{{#useOptional}}{{^required}}{{^isNullable}}{{^isContainer}}Optional.of({{/isContainer}}{{/isNullable}}{{/required}}{{/useOptional}}{{name}}{{#isNullable}}){{/isNullable}}{{#useOptional}}{{^required}}{{^isNullable}}{{^isContainer}}){{/isContainer}}{{/isNullable}}{{/required}}{{/useOptional}}; |
Contributor
There was a problem hiding this comment.
P1: Fluent setters now accept nullable input but still call Optional.of(...), so passing null can throw NullPointerException for optional properties.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/java-camel-server/pojo.mustache, line 158:
<comment>Fluent setters now accept nullable input but still call `Optional.of(...)`, so passing null can throw `NullPointerException` for optional properties.</comment>
<file context>
@@ -155,7 +155,7 @@ public class {{classname}}{{#parent}} extends {{{parent}}}{{/parent}}{{^parent}}
{{! begin feature: fluent setter methods }}
- public {{classname}} {{name}}({{{datatypeWithEnum}}} {{name}}) {
+ public {{classname}} {{name}}({{>nullableDataType}} {{name}}) {
{{#openApiNullable}}
this.{{name}} = {{#isNullable}}JsonNullable.of({{/isNullable}}{{#useOptional}}{{^required}}{{^isNullable}}{{^isContainer}}Optional.of({{/isContainer}}{{/isNullable}}{{/required}}{{/useOptional}}{{name}}{{#isNullable}}){{/isNullable}}{{#useOptional}}{{^required}}{{^isNullable}}{{^isContainer}}){{/isContainer}}{{/isNullable}}{{/required}}{{/useOptional}};
</file context>
Suggested change
| public {{classname}} {{name}}({{>nullableDataType}} {{name}}) { | |
| {{#openApiNullable}} | |
| this.{{name}} = {{#isNullable}}JsonNullable.of({{/isNullable}}{{#useOptional}}{{^required}}{{^isNullable}}{{^isContainer}}Optional.of({{/isContainer}}{{/isNullable}}{{/required}}{{/useOptional}}{{name}}{{#isNullable}}){{/isNullable}}{{#useOptional}}{{^required}}{{^isNullable}}{{^isContainer}}){{/isContainer}}{{/isNullable}}{{/required}}{{/useOptional}}; | |
| public {{classname}} {{name}}({{>nullableDataType}} {{name}}) { | |
| {{#openApiNullable}} | |
| this.{{name}} = {{#isNullable}}JsonNullable.of({{/isNullable}}{{#useOptional}}{{^required}}{{^isNullable}}{{^isContainer}}Optional.ofNullable({{/isContainer}}{{/isNullable}}{{/required}}{{/useOptional}}{{name}}{{#isNullable}}){{/isNullable}}{{#useOptional}}{{^required}}{{^isNullable}}{{^isContainer}}){{/isContainer}}{{/isNullable}}{{/required}}{{/useOptional}}; | |
| {{/openApiNullable}} |
| @@ -144,7 +144,7 @@ public {{>sealed}}class {{classname}}{{#parent}} extends {{{parent}}}{{/parent}} | |||
| {{^lombok.Data}} | |||
Contributor
There was a problem hiding this comment.
P1: Fluent setter parameters now use the wrapped nullable/optional type, but the body wraps the argument again, causing invalid nested JsonNullable/Optional assignments.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache, line 147:
<comment>Fluent setter parameters now use the wrapped nullable/optional type, but the body wraps the argument again, causing invalid nested `JsonNullable`/`Optional` assignments.</comment>
<file context>
@@ -144,7 +144,7 @@ public {{>sealed}}class {{classname}}{{#parent}} extends {{{parent}}}{{/parent}}
{{! begin feature: fluent setter methods }}
- public {{classname}} {{name}}({{>nullableAnnotation}}{{{datatypeWithEnum}}} {{name}}) {
+ public {{classname}} {{name}}({{>nullableAnnotation}}{{>nullableDataType}} {{name}}) {
{{#openApiNullable}}
this.{{name}} = {{#isNullable}}JsonNullable.of({{/isNullable}}{{#useOptional}}{{^required}}{{^isNullable}}{{^isContainer}}Optional.of{{#optionalAcceptNullable}}Nullable{{/optionalAcceptNullable}}({{/isContainer}}{{/isNullable}}{{/required}}{{/useOptional}}{{name}}{{#isNullable}}){{/isNullable}}{{#useOptional}}{{^required}}{{^isNullable}}{{^isContainer}}){{/isContainer}}{{/isNullable}}{{/required}}{{/useOptional}};
</file context>
Comment on lines
+304
to
307
| public AdditionalPropertiesClass anytype2(JsonNullable<Object> anytype2) { | ||
| this.anytype2 = JsonNullable.of(anytype2); | ||
| return this; | ||
| } |
Contributor
There was a problem hiding this comment.
P2: Fluent setter double-wraps JsonNullable input, losing undefined/present semantics for anytype2.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/spring-http-interface-useHttpServiceProxyFactoryInterfacesConfigurator/src/main/java/org/openapitools/model/AdditionalPropertiesClass.java, line 304:
<comment>Fluent setter double-wraps JsonNullable input, losing undefined/present semantics for anytype2.</comment>
<file context>
@@ -301,7 +301,7 @@ public void setAnytype1(@Nullable Object anytype1) {
}
- public AdditionalPropertiesClass anytype2(Object anytype2) {
+ public AdditionalPropertiesClass anytype2(JsonNullable<Object> anytype2) {
this.anytype2 = JsonNullable.of(anytype2);
return this;
</file context>
Suggested change
| public AdditionalPropertiesClass anytype2(JsonNullable<Object> anytype2) { | |
| this.anytype2 = JsonNullable.of(anytype2); | |
| return this; | |
| } | |
| public AdditionalPropertiesClass anytype2(JsonNullable<Object> anytype2) { | |
| this.anytype2 = anytype2; | |
| return this; | |
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #23681
{{{datatypeWithEnum}}} for fluent setter parameters
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)Summary by cubic
Adds missing JSpecify
@Nullableto Spring fluent setter parameters and preserves property order in auto-generated examples. Fixes #23681.{{>nullableDataType}}(with{{>nullableAnnotation}}) for fluent setter parameters inJavaSpring/pojo.mustacheand inherited setters; same change injava-camel-server/pojo.mustache. Ensures@Nullableis applied whenuseJspecifyis enabled.LinkedHashMapinExampleGeneratorto keep spec-defined property order in object examples; add unit test; update sampleopenapi.yamlfiles.Written for commit fb74567. Summary will update on new commits.